Migrates topic-viewer-navbar-breadcrumb, subtopics-list, topic-viewer-page, practice-tab, topic-viewer-stories-list to angular components and introduces angular2+ material css. #9957

srijanreddy98 · 2020-07-19T08:40:52Z

Overview

This PR fixes or fixes part of Migrate directives/controllers to Angular components #9749.
This PR does the following:

Initial implementation by @orthodoxparadox #9790.

Material CSS doc:
PR Doc: https://docs.google.com/document/d/1UoCOC7XNhCZrWIMPAoR5Xex28OYWzteqXrqCU9gRUHQ/edit?usp=sharing

Adds a CSS file for angular material.
This CSS shouldn't be modified as it is generated.

Steps to generate this file:
How I generated this:

Video: https://drive.google.com/file/d/1bRA0824CV6cDNYANcX2KT4skNilKdynh/view?usp=sharing

Clone angular components: git clone https://github.com/angular/components.git
cd components
cd src/material/core/theming/prebuilt/
code . (if you use vscode or open this folder in the code editor of your choice).
open any of the four files present in the folder.
Generate and copy a palette from http://mcg.mbitson.com/#!?oppia=%23009688 and paste it in the scss file.
Change md - mat and change primary and secondary variables in scss file.
Change $accent varibale value to be mat-pallete($mat-blue)
Install node-sass (npm i -g node-sass)
Run node-sass ./deeppurple-amber.scss oppia-material.css

Changes:

Before	After

Essential Checklist

The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
The linter/Karma presubmit checks have passed locally on your machine.
"Allow edits from maintainers" is checked. (See here for instructions on how to enable it.)
- This lets reviewers restart your CircleCI tests for you.
The PR is made from a branch that's not called "develop".

PR Pointers

Oppiabot will notify you when you don't add a PR_CHANGELOG label. If you are unable to do so, please @-mention a code owner (who will be in the Reviewers list), or ask on Gitter.
For what code owners will expect, see the Code Owner's wiki page.
Make sure your PR follows conventions in the style guide, otherwise this will lead to review delays
Never force push. If you do, your PR will be closed.

srijanreddy98 · 2020-07-30T05:08:16Z

@mschanteltc here are the screenshots:
Revision Tab:

Practice Tab:

WDYT?

mschanteltc · 2020-07-30T21:43:03Z

@mschanteltc here are the screenshots:
Revision Tab:

Practice Tab:

WDYT?

The Revision Tab looks great! Just a few more notes about the "Start" button in the Practice Tab:

Use medium weight font for "Start"
I can't tell what the measurement of the rounded corners are, but do they match that of the white container's? They still look too "rounded" to me.

srijanreddy98 · 2020-07-31T02:20:05Z

@mschanteltc I will change the font-weight/size but the border-radius is correct IMO. I checked with the current develop:

Develop	My Branch

oppiabot · 2020-08-03T06:35:05Z

Hi @srijanreddy98. Due to recent changes in the "develop" branch, this PR now has a merge conflict. Please follow this link if you need help resolving the conflict, so that the PR can be merged. Thanks!

srijanreddy98 · 2020-08-04T04:58:47Z

@mschanteltc PTAL!

mschanteltc · 2020-08-04T20:58:17Z

@mschanteltc PTAL!

The corner radius lgtm! Was there a change to the font weight? I might've missed it.

mschanteltc · 2020-08-04T20:59:49Z

@mschanteltc PTAL!

As long as the corner radius of the button matches the might white container, it LGTM! Was there a change to the font weight? I might've missed it.

srijanreddy98 · 2020-08-05T18:45:17Z

@mschanteltc I have updated the font size.

WDYT

mschanteltc · 2020-08-06T03:48:05Z

@mschanteltc I have updated the font size.

WDYT

Hmm I still don't see the medium weight in the "Start" font. It still looks normal to me. Does a "medium" weight exist in the font library?

srijanreddy98 · 2020-08-06T19:05:35Z

@mschanteltc how does it look now?

mschanteltc · 2020-08-06T19:54:32Z

@mschanteltc how does it look now?

Thanks for the update! The font weight looks good now. Is it possible we can space out the characters in "START" a little more so we can see a small gap in between each one?

DubeySandeep

Reviewed the code owner files and left two minor comments PTAL! (Feel free to merge this PR and fix one minor issue in any other PR!)

DubeySandeep · 2020-08-07T02:45:19Z

.github/CODEOWNERS

@@ -354,6 +355,7 @@
 /core/README.md @ankita240796
 /extensions/README.md @ankita240796
 /scripts/README.md @ankita240796
+/core/templates/css/README.md @srijanreddy98 @bansalnitish


Add in sorted order!

DubeySandeep · 2020-08-07T02:46:50Z

scripts/linters/general_purpose_linter.py

@@ -39,6 +39,7 @@
    'core/tests/release_sources/tmp_unzip.zip', 'scripts/linters/test_files/*',
    'core/tests/release_sources/tmp_unzip.tar.gz',
    'core/templates/combined-tests.spec.ts',
+    'core/templates/css/oppia-material.css',


Why we need this here?

I added it here to be ignored by the linter because it is a generated CSS file and it was raising a lot of lint issues.

srijanreddy98 · 2020-08-07T20:04:51Z

@mschanteltc PTAL!

mschanteltc · 2020-08-07T20:06:51Z

@mschanteltc PTAL!

LGTM!

mschanteltc · 2020-08-11T22:21:05Z

I missed this, but for the Lessons Tab, the Chapter names should be aligned when a new line starts. Can we fix this so that the textbox aligns with the line, while leaving enough room on the left for a checkmark? (mock)

seanlip · 2020-08-11T23:46:17Z

Hi @mschanteltc, this PR is already merged. Could you please file a new issue instead?

Thanks!

…-page, practice-tab, topic-viewer-stories-list to angular components and introduces angular2+ material css. (oppia#9957) * Migrated PracticeTabComponent * Fixed references * Migrated TopicViewerNavbarBreadcrumb * Added untracked files * Migrates StoriesListComponent * Fixed frontend coverage * Styling changes + component addition * Migrates subtopics list component * Fixed frontend coverage * Fixed ngfor * Fix html * Fix translates and return types for ngOnInit * Fixed coverage check * Removes translation services following PR oppia#9842 + other minor fixes * Removed fully covered files * Undo constants * Oppia-material * Don't lint oppia-mat css * Lint check fixes 😭 * Material * Merge branch 'angular-mat-css' into tv-migrate * Changes * Changes * Changes * Changes to shared components * fix main component file * Change subtopics component styling * Linter 😑 * Fully covered files 😩 * Update story summary * Fix layout issues * Files seem to fully cover themselves 😂 * Review * Review * Review * Review * Change button and checkbox styles * Update readme.ms * Review * Style changes * Style changes * Style changes * Style changes * Changes * Review Co-authored-by: Madhav Sainanee <msainanee@google.com> Co-authored-by: Madhav Sainanee <30462390+orthodoxparadox@users.noreply.github.com>

sainanee and others added 30 commits July 5, 2020 20:09

Migrated PracticeTabComponent

fb57a9b

Fixed references

7a0c88c

Migrated TopicViewerNavbarBreadcrumb

316e7cb

Added untracked files

91d7b6b

Migrates StoriesListComponent

410d43b

Fixed frontend coverage

122252c

Styling changes + component addition

8706628

Migrates subtopics list component

5488cba

Fixed frontend coverage

cc7c690

Fixed ngfor

5f0ac4c

Fix html

a46ca87

Fix translates and return types for ngOnInit

9be7c24

Fixed coverage check

fea29ec

Resolve conflicts + fixed return types

38d8c38

Merge remote-tracking branch 'upstream/develop' into tv-migrate

f3ac675

Removes translation services following PR oppia#9842 + other minor fixes

b70638a

Removed fully covered files

4b962ca

Resolve conflicts

cb07fcd

Merge branch 'develop' into tv-migrate

53e7a2d

Merge branch 'develop' into tv-migrate

38a934f

Review comments

b4eb4d0

Undo constants

884fb2d

Oppia-material

f8ea6a3

Merge remote-tracking branch 'upstream/develop' into angular-mat-css

71c2e83

Don't lint oppia-mat css

971ab07

Lint check fixes 😭

71e30d7

Material

904fd86

Merge with develop

8236051

Merge branch 'angular-mat-css' into tv-migrate

0dfabd3

Changes

aed63fc

srijanreddy98 added 3 commits July 30, 2020 10:43

Style changes

1bbc5b2

Style changes

40a7012

Style changes

bdf0591

oppiabot bot added the PR: don't merge - HAS MERGE CONFLICTS label Aug 3, 2020

Merge remote-tracking branch 'upstream/develop' into tv-migrate

8deaedf

Style changes

d87ed9a

oppiabot bot removed the PR: don't merge - HAS MERGE CONFLICTS label Aug 5, 2020

Changes

b911f33

srijanreddy98 requested a review from DubeySandeep August 7, 2020 02:45

srijanreddy98 assigned DubeySandeep Aug 7, 2020

DubeySandeep approved these changes Aug 7, 2020

View reviewed changes

DubeySandeep removed their assignment Aug 7, 2020

Review

b04a548

srijanreddy98 merged commit 9138dfd into oppia:develop Aug 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrates topic-viewer-navbar-breadcrumb, subtopics-list, topic-viewer-page, practice-tab, topic-viewer-stories-list to angular components and introduces angular2+ material css. #9957

Migrates topic-viewer-navbar-breadcrumb, subtopics-list, topic-viewer-page, practice-tab, topic-viewer-stories-list to angular components and introduces angular2+ material css. #9957

srijanreddy98 commented Jul 19, 2020 •

edited

Loading

srijanreddy98 commented Jul 30, 2020

mschanteltc commented Jul 30, 2020

srijanreddy98 commented Jul 31, 2020

oppiabot bot commented Aug 3, 2020

srijanreddy98 commented Aug 4, 2020

mschanteltc commented Aug 4, 2020

mschanteltc commented Aug 4, 2020

srijanreddy98 commented Aug 5, 2020

mschanteltc commented Aug 6, 2020

srijanreddy98 commented Aug 6, 2020

mschanteltc commented Aug 6, 2020

DubeySandeep left a comment

DubeySandeep Aug 7, 2020

srijanreddy98 Aug 7, 2020

DubeySandeep Aug 7, 2020

srijanreddy98 Aug 7, 2020

srijanreddy98 commented Aug 7, 2020

mschanteltc commented Aug 7, 2020

mschanteltc commented Aug 11, 2020

seanlip commented Aug 11, 2020

Migrates topic-viewer-navbar-breadcrumb, subtopics-list, topic-viewer-page, practice-tab, topic-viewer-stories-list to angular components and introduces angular2+ material css. #9957

Migrates topic-viewer-navbar-breadcrumb, subtopics-list, topic-viewer-page, practice-tab, topic-viewer-stories-list to angular components and introduces angular2+ material css. #9957

Conversation

srijanreddy98 commented Jul 19, 2020 • edited Loading

Overview

Essential Checklist

PR Pointers

srijanreddy98 commented Jul 30, 2020

mschanteltc commented Jul 30, 2020

srijanreddy98 commented Jul 31, 2020

oppiabot bot commented Aug 3, 2020

srijanreddy98 commented Aug 4, 2020

mschanteltc commented Aug 4, 2020

mschanteltc commented Aug 4, 2020

srijanreddy98 commented Aug 5, 2020

mschanteltc commented Aug 6, 2020

srijanreddy98 commented Aug 6, 2020

mschanteltc commented Aug 6, 2020

DubeySandeep left a comment

Choose a reason for hiding this comment

DubeySandeep Aug 7, 2020

Choose a reason for hiding this comment

srijanreddy98 Aug 7, 2020

Choose a reason for hiding this comment

DubeySandeep Aug 7, 2020

Choose a reason for hiding this comment

srijanreddy98 Aug 7, 2020

Choose a reason for hiding this comment

srijanreddy98 commented Aug 7, 2020

mschanteltc commented Aug 7, 2020

mschanteltc commented Aug 11, 2020

seanlip commented Aug 11, 2020

srijanreddy98 commented Jul 19, 2020 •

edited

Loading